Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce pub exposure in rustc_mir_build #125959

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

nnethercote
Copy link
Contributor

r? compiler

Exhaustiveness and usefulness checking are now in
`rustc_pattern_analysis`.
A lot of errors don't need to be visible outside the crate, and some
other things as well.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 4, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 4, 2024

Some changes occurred in match lowering

cc @Nadrieril

Copy link
Member

@Urgau Urgau Jun 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many errors.rs/lints.rs files have this kind of structure: public diagnostic struct with public fields.
Do you think it would make sense to deny the unreachable_pub lint in those files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the first I've heard of this lint! If it's reliable, I think we should use it just about everywhere :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know it is reliable and the only reason it isn't warn-by-default yet is because of pub-fields in non-pub-adt, cf. #120144 (comment).

@compiler-errors
Copy link
Member

Would like to see another PR changing (almost?) all diagnostic structs to be pub(crate).

This is good for now @bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 4, 2024

📌 Commit 5a5e248 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 4, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 4, 2024
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125667 (Silence follow-up errors directly based on error types and regions)
 - rust-lang#125717 (Refactor `#[diagnostic::do_not_recommend]` support)
 - rust-lang#125795 (Improve renaming suggestion for names with leading underscores)
 - rust-lang#125865 (Fix ICE caused by ignoring EffectVars in type inference)
 - rust-lang#125953 (Streamline `nested` calls.)
 - rust-lang#125959 (Reduce `pub` exposure in `rustc_mir_build`)
 - rust-lang#125967 (Split smir `Const` into `TyConst` and `MirConst`)
 - rust-lang#125968 (Store the types of `ty::Expr` arguments in the `ty::Expr`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8272c6d into rust-lang:master Jun 4, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone Jun 4, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 4, 2024
Rollup merge of rust-lang#125959 - nnethercote:rustc_mir_build-cleanups, r=compiler-errors

Reduce `pub` exposure in `rustc_mir_build`

r? compiler
@nnethercote nnethercote deleted the rustc_mir_build-cleanups branch June 4, 2024 21:10
@nnethercote
Copy link
Contributor Author

I have started adding #![deny(unreachable_pub)] to every crate in compiler. I've done about a dozen crates so far, it's going well. Diagnostic structs are definitely the most common case, but it's finding other stuff as well. I will file a PR later today for maybe the first 20 crates or so.

@estebank
Copy link
Contributor

estebank commented Jun 5, 2024

@nnethercote on the one hand, I'm glad you're doing that for the sake of the codebase, on the other I have a project that uses the rustc rlibs to build its own rustc_driver and there might be "unused pub items" that I might be using. At least two other tools might be silently affected by that too: miri and klint. That doesn't mean we shouldn't enable that lint, just that you should be aware that people might mildly grumble after the clean up.

@nnethercote
Copy link
Contributor Author

@estebank: I am familiar with that general problem, and there are now comments on several seemingly unused things in rustc_interface that came about when I (a) tried to remove/hide them, or (b) removed/hid them and then undid that after a complaint :)

But unreachable_pub shouldn't have that problem. It only complains about things marked pub that can't actually be exported because they're within something else (e.g. a struct, a mod) that is itself not pub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants